CAMEL-25823: camel-core - Start Split and Multicast output producers when the route starts - #28147

TL;DR

Moving the error-handler start to doStart() fixes the reported Split/Multicast symptom. But gating the runtime start on the CamelContext status leaves error handlers built at runtime unstarted. That breaks Recipient List, which did not have the reported bug, while the context is starting or stopping.

Blockers

• MulticastProcessor.java:1220: the getCamelContext().isStarted() gate also skips the start of error handlers built at runtime.

Other findings: 5 (non-blocking)

Blocker: error handlers built at runtime stay unstarted

isStarted() is true only in Started (BaseService.java:319-321). So it is false in more phases than doInit(), where the context is Initializing. It is also false while the context is Starting: with the default route controller, each route's consumer runs before the next route starts (InternalRouteStartupManager.java:425,465). And it is false while the context is Stopping: in-flight exchanges finish after stop() has set Stopping (BaseService.java:173, AbstractCamelContext.java:3343).

Split and Multicast pre-build their wrappers in doInit(), so the new doStart() loop covers them. RecipientListProcessor, however, is created with route == null (RecipientList.java:218-219) and builds every wrapper at runtime (RecipientListProcessor.java:370). For a recipient first seen in one of those phases, the DefaultErrorHandler clone is never started. Its taskFactory is only created in doStart() (RedeliveryErrorHandler.java:2159-2168), so process() fails the sub-exchange with a NullPointerException (:191, :200-203). The wrapper is also cached (MulticastProcessor.java:1214), and the cache-hit path returns it without starting it (:1200-1203). So that recipient keeps failing until the route restarts. Before this PR the start was unconditional.

What do you think about gating on this processor's own state instead, which stays false until the processor starts?

if (isRunAllowed()) {
    ServiceHelper.startService(answer);
}

A test that sends through a recipientList while the context is still Starting would cover this, for example from an event notifier on that route's RouteStartedEvent.

Other findings

1. Upgrade guide (non-blocking). Users will notice this change at startup:

   • With the default route controller, context.start() now fails with a FailedToStartRouteException for the route (RouteService.java:126-132) instead of the producer's own exception.
   • These producers now start with their route, in startupOrder, instead of ahead of all routes.
   • With the supervising controller, the producers of an autoStartup=false route are no longer started with the context.

   A short camel-core note in camel-4x-upgrade-guide-4_24.adoc would help, for example after the pollEnrich note.

2. Tests (suggestion). DefaultSupervisingRouteControllerMulticastSplitTest only checks the route status, so it also passes without the doStart() hunk. Sending a message once the route is Started and checking that the endpoint saw three start attempts would cover the retry path. Comparing with ServiceStatus.Started avoids the toString() comparison.

3. Follow-up, pre-existing. doStop() and doShutdown() (MulticastProcessor.java:1311,1316) pass the errorHandlers map. ServiceHelper only handles Service and Iterable (ServiceHelper.java:185-191), so for a map it does nothing, and the wrappers started at :1288 are never stopped. Producers are still stopped through processors (SendProcessor.doStop), so nothing leaks. Passing errorHandlers.values() would make the lifecycle symmetric.

4. Follow-up, pre-existing. Splitter.doInit() (Splitter.java:130-133) still starts the watermark resumeStrategy and loads its cache during route init. A broker-backed strategy that cannot connect yet would fail the context in the same way. This is worth its own Jira.

5. Title (suggestion). The PR title becomes the squash commit subject and describes the bug, so in the log it reads as the opposite of the change. Something like "start the Split and Multicast outputs when the route starts" would describe the fix.

Checked and fine:

• The per-output producers are still started by doStart() through processors (:1284).
• errorHandlers.values() is a snapshot (SimpleLRUCache.java:314-316), and BaseService.start() does nothing when the service is already started (BaseService.java:116-123), so there is no double start.
• There is no change to ProducerCache or cacheSize.

Reviewed with Claude Code on behalf of oscerd. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

Following up on the prior review (posted under oscerd) — the second commit addresses most of the raised points.

What's addressed:

• ✅ Blocker: isRunAllowed() gate replaces getCamelContext().isStarted(). isRunAllowed() returns true in STARTING/STARTED/SUSPENDED states, so error handlers built at runtime during Starting (e.g. RecipientListProcessor) are correctly started immediately. During doInit() (INITIALIZING state), isRunAllowed() is false, so wrappers are deferred to doStart() as intended.
• ✅ doStop()/doShutdown() now pass errorHandlers.values() — fixes the silent no-op that left error handler wrappers never stopped.
• ✅ Upgrade guide section added with accurate description of the three behavioral changes.
• ✅ Tests now assert start attempt counts (assertEquals(3, endpoint.startAttempts.get())) and message processing counts — and use ServiceStatus.Started for the status comparison.
• ✅ PR title updated to describe the fix, not the bug.

One test bug remains:

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Following up on my earlier review — all issues have been addressed.

What's fixed in the 3rd commit (0fd6dbe):

• ✅ Test delimiter bug: template.sendBody("direct:split", "a b c") → "a,b,c" — the space-delimited body produced a singleton under split(body())'s default comma delimiter; the processCount assertion would have always failed.

All prior findings (oscerd + gnodet-bot) are resolved:

• ✅ isRunAllowed() gate replaces getCamelContext().isStarted() — correctly covers STARTING/STARTED/SUSPENDING/SUSPENDED phases, so error handlers built at runtime by RecipientListProcessor are started immediately, while those built during doInit() (INITIALIZING phase) are deferred to doStart().
• ✅ doStart() starts pre-built error handlers via ServiceHelper.startService(errorHandlers.values()) — idempotent (no-op if already started).
• ✅ doStop()/doShutdown() now pass errorHandlers.values() — the prior code passing the Map directly was a silent no-op.
• ✅ Upgrade guide documents all three behavioral changes accurately.
• ✅ Test assertions strengthened: startAttempts == 3 validates the retry count, processCount validates end-to-end routing. Both gate correctly on the fix being present.

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Thanks Guillaume. Deferring the wrapper start to doStart() is the right fix for the supervising controller, and gating on the processor's own state (isRunAllowed()) addresses Andrea's recipientList concern. I went through the other users of MulticastProcessor: RecipientListProcessor (one instance per RecipientList, started in its doStart()), the SEDA multipleConsumers multicast (started before use) and Splitter (delegates to super). They are fine. Actually stopping the wrappers now (via values()) is safe too: RedeliveryErrorHandler has no doStop() and NoErrorHandler.doStop() is a no-op, so outputs and the shared dead letter processor are not stopped on a route stop, and doStart() starts the wrappers again on restart.

Things to address:

1. Blocking: the wrapper is now put in the cache before it is started, which opens a race for wrappers built at runtime (see inline).
2. Lines 1313 and 1318 are over the formatter's 128-character limit, so the build will reformat them and the uncommitted-changes check will fail. Please run mvn formatter:format impsort:sort in core/camel-core-processor.
3. Two of the upgrade guide bullets do not match what happens (see inline).
4. The PR description still describes the getCamelContext().isStarted() gate. Please update it to the isRunAllowed() version.

Optional: a test that sends through a recipientList to a recipient not seen before, from a few threads at once, would cover the runtime path that this change touches.

This review was generated by an AI agent (Claude Code on behalf of davsclaus) and may contain inaccuracies. Please verify all suggestions before applying.

Following up on my earlier review and davsclaus's review — all issues raised by both have been addressed in the 4th commit.

davsclaus findings resolved:

• ✅ Cache-before-start race: ServiceHelper.startService(answer) is now called before errorHandlers.putIfAbsent(key, answer), restoring the original guarantee that no concurrent exchange can observe an unstarted wrapper via the cache-hit path. The comment documents this explicitly.
• ✅ Line length: doStop() and doShutdown() calls are now wrapped across multiple lines (well within the 128-char formatter limit).
• ✅ Upgrade guide bullet 1 reframed: now focuses on when producers start rather than error behavior, with the init-only context example davsclaus suggested.
• ✅ Upgrade guide bullet 3: autoStartup=false note now correctly says "regardless of which route controller is used".
• ✅ PR description updated to describe the isRunAllowed() gate.

Minor nit (non-blocking): The inline comment says isRunAllowed() covers "STARTING/STARTED/SUSPENDED phases" but the actual implementation (status >= STARTING && status <= SUSPENDED) also includes SUSPENDING. The gate behavior is correct; only the comment is slightly incomplete.

LGTM. Solid fix for a tricky lifecycle ordering issue.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Previous findings are all addressed. The nit I raised in the last review (comment missing SUSPENDING phase) is fixed in commit f783b705.

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Re-review of 0fd6dbe..f783b70. Thanks Guillaume, all my points are addressed.

1. The wrapper is now started before it is put into errorHandlers, in the same order as on main, so the cache-hit path can no longer hand out an unstarted wrapper built at runtime. Wrappers built in doInit() are still cached unstarted and started in doStart(). Two threads that miss the cache for the same new recipient both start a wrapper and one wins putIfAbsent; the other is used for its own exchange and then dropped, which is unchanged from main and harmless, since starting a RedeliveryErrorHandler only builds its task factory.
2. The doStop()/doShutdown() lines are wrapped within the formatter limit.
3. The upgrade guide bullets now describe the actual change: the producers start with the route instead of at context initialization, and the autoStartup=false note applies with either route controller.
4. The PR description now describes the isRunAllowed() gate. Small leftovers, if you touch it again: it omits SUSPENDING from the phases, and it does not mention the errorHandlers.values() stop/shutdown fix or the upgrade-guide entry.

LGTM once CI is green.

This review was generated by an AI agent (Claude Code on behalf of davsclaus) and may contain inaccuracies. Please verify all suggestions before applying.

Previous findings are all addressed. The new commit (895e821) fixes the cut-off WARN regression cleanly.

What was fixed:

• ✅ RedeliveryErrorHandler.runNotAllowed(): adds logCutOffByRouteStop() call when the exchange already carries a RouteStoppingException and is not marked as interrupted — this restores the WARN that was dropped when doStop() began correctly stopping cached error handlers. The !isInterrupted() guard correctly avoids double-logging for interrupted sub-exchanges, which are already logged through their own path.

Pre-fix validation (static): ⚠️ Dynamic check skipped (build takes too long for CI-pending PR). The pre-existing RouteStopCutOffLogTest.theCutOffIsOneWarnLine() exercises exactly this scenario: a Split route is stopped while an exchange is inflight; the test asserts exactly one WARN line and no exhausted failure. Per gnodet's PR comment, this test was regressing before the fix.

No new issues found. The condition is sound, logCutOffByRouteStop guards internally with isCutOffByRouteStop(e) so it's a no-op for non-RouteStoppingException exceptions, and the call happens before taskFactory.release()/cb.done() (correct ordering).

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

TL;DR

Comment: my approval stands for the rest. Two questions on the new commit 895e821244, which logs the cut-off WARN in SimpleTask.runNotAllowed().

Blockers

• None, but please check the null logger below.

Other findings: 2 questions, 1 nit.

• RedeliveryErrorHandler.java:868: logger.getLevel() has no null check. LegacyDefaultErrorHandlerReifier.java:46 passes a null logger when none is configured, and logFailedDelivery guards logger == null for that reason. Add logger != null to the condition.
• Can it log more than once? Each nested EIP has its own channel error handler, and after a forced stop an exchange on its way out can pass several stopped ones. With an exchange that is not interrupted and keeps its RouteStoppingException, each of them now logs. For example, the theCutOffIsOneWarnLine route with the split inside a filter, or an inner handler that already logged through logFailedDelivery. I have reasoned this from the code and not run it. Marking the exchange after logging (as DirectProducer does with setInterrupted(true)), or skipping exhausted exchanges, plus a nested variant of the test asserting one WARN line, would settle it.
• Nit: the comment says "this is the route-level exchange", but the branch runs in any stopped error handler. Also, please add a line about this change to the PR description.

Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Following up on my earlier review and davsclaus's comment on commit 895e821244 — all three points raised are addressed by the new commit (41c171162).

davsclaus findings resolved:

• ✅ Null-logger guard: logger != null added to the condition in runNotAllowed() — matches the existing guard in logFailedDelivery, correctly protects against LegacyDefaultErrorHandlerReifier passing null.
• ✅ Single-WARN guarantee: exchange.getExchangeExtension().setInterrupted(true) is now set after the first logCutOffByRouteStop() call, ensuring that nested stopped error handlers (e.g. a split inside a filter) do not each emit their own WARN for the same exchange. The !isInterrupted() guard in the condition makes this self-consistent.
• ✅ Comment accuracy: the comment no longer says "route-level exchange" — it correctly describes the branch as running in any stopped error handler, not just the route-level one.
• ✅ PR description updated: section 4 ("Null-logger guard and single-WARN guarantee") was added.

Pre-fix validation (static): ⚠️ Dynamic check skipped (build time). Static trace confirms the condition is sound: the outer logger != null prevents NPE in the logger.getLevel() call; the !isInterrupted() guard prevents double-logging; logCutOffByRouteStop() is a no-op for non-RouteStoppingException exceptions. The existing theCutOffIsOneWarnLine test covers exactly this scenario.

Previous findings are all addressed. LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Following up on davsclaus's latest review — all three points are addressed in commit 41c171162c96.

davsclaus findings resolved:

• ✅ Null-logger guard: logger != null added to the condition at line 863 of runNotAllowed(), matching the same guard in logFailedDelivery (line 990). Avoids NPE when LegacyDefaultErrorHandlerReifier passes a null logger.
• ✅ Double-logging concern: exchange.getExchangeExtension().setInterrupted(true) is called immediately after logging the cut-off WARN. Subsequent stopped error handlers (e.g. in a split-inside-filter scenario) see isInterrupted() == true and skip the branch entirely — only one WARN is emitted per exchange. The existing theCutOffIsOneWarnLine test validates the single-WARN guarantee; a nested variant would add more confidence but is left optional per the review.
• ✅ Comment nit: the comment no longer says "this is the route-level exchange" — it now reads "the exchange is passing through a stopped error handler" which is accurate for nested EIP handlers as well.
• ✅ PR description: updated to describe the null-logger guard and the setInterrupted(true) guarantee (item 4 in the Fix section).

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Re-review of 895e821..41c1711. Thanks Guillaume. The null-logger guard (RedeliveryErrorHandler.java:863) and the comment are fixed, and the description now covers this change. My approval of the Multicast/Split part stands; it is unchanged since f783b70.

One question about RedeliveryErrorHandler.java:869-870 before I approve again: setInterrupted(true) is more than a log marker. A running error handler checks it first (run(), line 466) and takes runInterrupted(), which skips onException, redelivery and the dead letter channel and keeps the exception.

Example: route A (running, with a DLC) calls direct:b, and route B is force-stopped mid-exchange.

• Before: A's error handler handled the RouteStoppingException (DLC, one WARN via logFailedDelivery).
• Now: B's stopped handler logs and marks the shared exchange, so A skips the DLC and its consumer rolls back.

That may be the intended CAMEL-25502 meaning (DirectProducer does the same), but here it comes from deduplicating a log line, and it only happens when the handler has a logger. Could we either use a dedicated "cut-off logged" marker, or keep this behaviour and state it in the description and the 4.24 upgrade guide, with a test for the calling-route case? I traced this from the code and have not run it.

Optional: copyResults/MulticastProcessor:972 do not carry interrupted from a sub-exchange to the parent. A cut-off that does not start as interrupted (a slow .process() inside the split rather than the direct wait) may therefore log once for the sub-exchange and once for the parent. RouteStopCutOffLogTest.theCutOffIsOneWarnLine only covers the DirectProducer path, and it counts after the first message arrives. A variant that waits for the route to stop before counting would settle it.

Reviewed with Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

Following up on davsclaus's latest review — all four points are addressed in this commit.

• ✅ Upgrade guide narrowness: the note now correctly lists all stopped error handlers (split, multicast, filter, choice, ...) instead of "Split or Multicast".
• ✅ Wrong API in doc: the upgrade guide now uses exchange.getExchangeExtension().setInterrupted(true) correctly.
• ✅ Caller-route test: RouteStopCutOffCallerRouteTest added, covering the DLC-not-called behavior davsclaus requested.
• ✅ Flaky inflight-detection: RouteStopCutOffLogNestedTest changed from e.getNodeId() to e.getToUri() for the await condition.

One nit in the new test (inline).

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Following up on my earlier review — all previous findings are resolved.

New commits since last review:

• 39d7b21 — Merge upstream/main (routine rebase, no code changes)
• db0c911 — Fix flaky inflight detection in RouteStopCutOffCallerRouteTest

db0c911 assessment:
✅ Flaky inflight detection fix: e.getNodeId() → e.getToUri() — identical to the fix applied to RouteStopCutOffLogNestedTest in the prior commit. getToUri() is set as soon as DirectProducer picks up the exchange, while getNodeId() may not be set on a slow agent. Correct and consistent.

Remaining status:

• ✅ All davsclaus findings from the Oct 25 review are addressed (upgrade guide broadened, caller-route DLC test added, flaky-test detection fixed)
• ✅ All code in MulticastProcessor.java and RedeliveryErrorHandler.java unchanged since davsclaus's approval
• ✅ No new issues introduced

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Following up on my earlier review — all previous findings resolved.

New commit (9c49594):
✅ getToUri() → getNodeId() correction — my previous suggestion referenced getToUri(), which does not exist on InflightRepository.InflightExchange (only getNodeId() is exposed). The author correctly reverted to getNodeId(), which matches the .id("shipment") annotation on the direct:shipment step and is already used consistently in RouteStopCutOffLogNestedTest. The fix is correct.

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Following up on davsclaus's Oct 25 review — all 4 points are addressed.

davsclaus findings resolved:

• ✅ Upgrade guide scope: the note now lists all stopped EIP handlers (split, multicast, filter, choice, ...) instead of just "Split or Multicast". The correct API (exchange.getExchangeExtension().setInterrupted(true)) is used. The calling-route consequence (caller route skips onException, redelivery, and DLC) is fully documented, matching davsclaus's suggested wording.
• ✅ Caller-route test: RouteStopCutOffCallerRouteTest.callerRouteDlcNotCalledOnNestedEipCutOff() pins down that route A's DLC mock is NOT called when route B's nested EIP is force-stopped. This exactly covers the scenario davsclaus requested.
• ✅ Flaky test fix: RouteStopCutOffLogNestedTest now waits for both getRouteStatus("nested").isStopped() and getInflightRepository().browse().isEmpty() before counting WARNs — the exact fix davsclaus asked for. The getNodeId() condition is also correct (.id("shipment") annotates the direct:shipment step).
• ✅ Regen commit (ba04753): purely Javadoc line-wrapping reformatting in RouteStopCutOffCallerRouteTest.java. Zero code changes.

One remaining latent gap (non-blocking, pre-existing): ExchangeHelper.copyResults() does not propagate the interrupted flag from sub-exchanges to their parent. If a slow .process() step (rather than a DirectProducer wait) is cut off inside a nested EIP, the route-level error handler may emit a second WARN because the original exchange lacks the interrupted flag set on the sub-exchange. This was davsclaus's optional concern from Oct 25 — it's a pre-existing limitation of copyResults, not a regression from this PR, and is cosmetic (double WARN, no data-loss risk).

LGTM. The lifecycle fix (MulticastProcessor/RedeliveryErrorHandler) is solid and all review concerns are addressed.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Re-review of c6d590d090..ba0475349e. Thanks Guillaume. The upgrade-guide paragraph (camel-4x-upgrade-guide-4_24.adoc:352-361) now describes the change as it really is: any nested EIP of the stopped route, the right getExchangeExtension().setInterrupted(true) API, and the effect on a calling route. MulticastProcessor and RedeliveryErrorHandler are unchanged, the merge of main did not touch the PR's own changes, and the Regen commit only reflows the new test's javadoc. One problem is left before I approve: I don't think the new caller-route test can pass yet.

1. RouteStopCutOffCallerRouteTest waits for an interrupt that never comes (:56-72). Route B is consumed from direct:b, so the thread blocked on direct:shipment is route A's seda:caller consumer thread, waiting in DirectComponent.getConsumer(). DirectProducer only turns that wait into a RouteStoppingException when the thread is interrupted. A forced stop of the direct-consumed route B only stops its DirectConsumer, and nothing interrupts route A's thread. The existing cut-off tests work because they stop the seda route itself, which shuts down its executor. So I expect the exchange to stay inflight for the 30 s direct timeout and the 5 s wait at :66-68 to time out. After that the sub-exchange fails with DirectConsumerNotAvailableException rather than a RouteStoppingException, and route A's DLC would get it. The test has not run in CI yet: db0c911 failed to compile on getToUri(), 9c49594 stopped at the regen check, and the run on ba04753 is still building.

Also, expectedMessageCount(0) plus assertIsSatisfied() passes at once, so the test never checks what actually happened to the exchange. Could we make it deterministic and positive? For example:

• route A is from("direct:caller") with deadLetterChannel("mock:dlc")
• in route B, inside multicast().split(), a .process() counts down an entered latch and then waits on a release latch, with a timeout
• the test calls template.asyncSend("direct:caller", ...), waits for entered, calls stopRoute("nested-b"), then releases the latch
• then it asserts that f.get(10, SECONDS) holds a RouteStoppingException and is interrupted, and that mock:dlc received nothing

That also covers the cut-off of a slow .process() that does not start out interrupted, which I asked about before.

2. The nested test wait (RouteStopCutOffLogNestedTest.java:82-86). Waiting for the inflight repository to drain before counting fixes the early count, thanks. It does not explain the flake we actually saw (Run 1 of 40092170771: no WARN at all within 5 s); with this change that case fails as assertEquals(1, 0) instead. The stricter wait may also expose a second WARN line: the interrupted flag is not carried from a sub-exchange to its parent (MulticastProcessor:972/977, ExchangeHelper.doCopyResults), so the multicast's stopped handler may log again after the split's handler. The current CI run will tell us.

3. Optional. The paragraph at :352 sits under "Split and Multicast child producers start with their route" (:341), but it applies to any nested EIP. Its own === camel-core - ... heading would make it easier to find.

Claude Code on behalf of Claus Ibsen. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

Previous findings are all addressed. The new test design is correct and sound — the latch + interrupt approach eliminates the flakiness that davsclaus identified. Two minor robustness nits on the test.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Previous findings are all addressed. The two nits from my last review are fixed in commit bc13f497.

Nit 1 — assertTrue on latch await: ✅ assertTrue(entered.await(5, TimeUnit.SECONDS), "Processor did not enter within 5 seconds") — the return value is now checked with a clear failure message, exactly as suggested.

Nit 2 — TimeoutException not caught: ✅ } catch (java.util.concurrent.TimeoutException e) { fail("Exchange did not complete within 10 seconds — likely a hang in the error handler chain", e); } — the timeout case now fails with a diagnostic message rather than propagating as an uncaught checked exception.

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Re-review of ba0475349e..bc13f497e3. Thanks Guillaume. Only RouteStopCutOffCallerRouteTest changed; MulticastProcessor and RedeliveryErrorHandler are unchanged. The previous CI failure (:66 ConditionTimeout on all 3 attempts) was the direct-consumer stop that never interrupts route A's thread. The new version gets its interrupt from route A's seda executor (SedaConsumer.doStop() -> shutdownNow), so I expect it to pass.

One thing is left before I approve: the test no longer tests the behaviour the upgrade guide and its own javadoc describe.

1. The caller route is stopped too, so the test can't tell why the DLC was skipped (RouteStopCutOffCallerRouteTest.java:88). The class javadoc (:38) and the 4.24 upgrade-guide paragraph are about "a calling route A that is still running". The test now calls stopRoute("caller-a"), which stops route A's channel error handler. A stopped RedeliveryTask skips the DLC on its own (run() -> !isRunAllowed(), RedeliveryErrorHandler.java:1131-1138). So the assertion at :104 passes whether or not the interrupted flag did its job.

2. The test throws the RouteStoppingException itself (:132). The framework never turns a user processor's interrupt into a RouteStoppingException, so the test checks the marker logic only for an exception it made itself.

Could we keep route A running and let the framework create the cut-off? For example:

• route A stays from("seda:caller") (or direct:caller) with deadLetterChannel("mock:dlc"), and is not stopped
• inside B's multicast().split(): a .process() that counts down entered and then waits on a release latch (with a timeout), followed by .to("mock:afterBlock")
• the test calls assertTrue(entered.await(5, SECONDS)), then stopRoute("nested-b"), then release.countDown()

The stopped channel of mock:afterBlock then sets the RouteStoppingException in runNotAllowed(), and the running route A should take the interrupted path. Then assert the RouteStoppingException, that mock:dlc is empty, and that mock:afterBlock received nothing. This also removes the Thread.sleep(Long.MAX_VALUE) at :129 (CLAUDE.md: no Thread.sleep() in new tests; a latch await is interruptible too).

3. Small things.

• Thanks for checking the entered.await(...) result now (:79).
• :99 catches java.util.concurrent.TimeoutException by its fully qualified name; please import it instead (CLAUDE.md "Import Style").
• Optional: the interrupted paragraph still sits under the Split/Multicast heading in the upgrade guide.

Blast radius. I grepped core, components and dsl for tests that need Split/Multicast producers started at context initialization. The only hits are autoStartup(false) routes (sql, stream, solr, file split tests, SplitterStreamingUoWIssueTest), and each starts its route before sending. Nothing depends on the old behaviour.

@oscerd: your CHANGES_REQUESTED from 2026-10-21 is still the blocking state. Your blocker is addressed (isRunAllowed() is the gate at MulticastProcessor.java:1217, and the wrapper is started before it is cached), and findings 1, 2, 3 and 5 are in the PR too. Could you re-review or dismiss it? The Splitter resumeStrategy follow-up (your 4) has no JIRA yet as far as I can see.

Reviewed with Claude Code on behalf of @davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

Following up on davsclaus's Oct 25 08:52 review — all three points are addressed in commit ef115368.

davsclaus findings resolved:

• ✅ Caller route stays running: route A (caller-a) is no longer stopped. Only stopRoute("nested-b") is called, so route A remains live throughout. A 1 s shutdown timeout is set so that stopRoute("nested-b") force-stops after 1 s (the direct:b consumer owns no thread of its own; the seda:caller executor thread is NOT interrupted by the force-stop). After stopRoute returns, MulticastProcessor.doStop() has already stopped the nested error-handler wrappers.
• ✅ Framework creates the RouteStoppingException: the processor no longer throws it manually. The processor counts down entered, then blocks on a release latch and exits normally. After the latch is released, the next step (mock:afterBlock) is in a stopped error-handler wrapper that invokes runNotAllowed(), which sets the RouteStoppingException on the exchange and calls setInterrupted(true) — exactly the path the upgrade guide and the test javadoc describe.
• ✅ TimeoutException imported: java.util.concurrent.TimeoutException is now imported; the fully-qualified name in the catch clause is gone.

What the test now actually verifies: the interrupted flag set by the nested stopped wrapper propagates back to the running route A's error handler (runInterrupted() → skips DLC), which is the documented behaviour the class javadoc and the 4.24 upgrade-guide paragraph describe.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Following up on davsclaus's Oct 25 08:52 review — all three points are addressed in commit ef115368.

davsclaus findings resolved:

• ✅ Caller route stays running: route A (caller-a) is no longer stopped. Only stopRoute("nested-b") is called, so route A remains live throughout. A 1 s shutdown timeout is set so that stopRoute("nested-b") force-stops after 1 s (the direct:b consumer owns no thread of its own; the seda:caller executor thread is NOT interrupted by the force-stop). After stopRoute returns, MulticastProcessor.doStop() has already stopped the nested error-handler wrappers.
• ✅ Framework creates the RouteStoppingException: the processor no longer throws it manually. The processor counts down entered, then blocks on a release latch and exits normally. After the latch is released, the next step (mock:afterBlock) is in a stopped error-handler wrapper that invokes runNotAllowed(), which sets the RouteStoppingException on the exchange and calls setInterrupted(true) — exactly the path the upgrade guide and the test javadoc describe.
• ✅ TimeoutException imported: java.util.concurrent.TimeoutException is now imported; the fully-qualified name in the catch clause is gone.

What the test now actually verifies: the interrupted flag set by the nested stopped wrapper propagates back to the running route A's error handler (runInterrupted() → skips DLC), which is the documented behaviour the class javadoc and the 4.24 upgrade-guide paragraph describe.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Re-review of bc13f497e3..ef11536884. Thanks Guillaume. Only the caller test and the upgrade guide changed, and MulticastProcessor and RedeliveryErrorHandler are as before, so my approval of that part stands.

The caller test now does what I asked:

• Route A stays running, and route B's stopped handlers create the RouteStoppingException themselves.
• stopRoute returns only after stopRouteService has stopped B's handlers (AbstractCamelContext.java:1516-1519), so releasing the latch afterwards is deterministic.
• Route A then ends in isDone() on the interrupted flag (RedeliveryErrorHandler.java:302-310).
• The test really checks something: if the DLC had handled the exchange, the future would succeed and the test would fail.
• The Thread.sleep and the FQCN are gone, and the interrupted paragraph has its own heading.

One thing is left before I approve:

1. RouteStopCutOffLogNestedTest flaked again.

• On this head, Run 1 failed at :89 with [] ==> expected: <1> but was: <0> and passed on the rerun (run 40179096494, incremental-test / flakes artifacts).
• That is two failed first attempts in the last three runs that reached the tests (c6d590d090 and ef11536884).
• I think the cause is the stop order. A forced stopRoute first interrupts the seda thread in shutdownRoutesNow (DefaultShutdownStrategy.java:255). Only afterwards does stopRouteService stop the route's error handlers (AbstractCamelContext.java:1519). So the interrupted exchange unwinds through handlers that may or may not be stopped yet.
• The latch shape the caller test now uses avoids that: block in a .process(), call stopRoute, then release. RouteStopCutOffLogTest already covers the DirectProducer wait.
• The test is new in this PR, so it is best fixed here.

2. Optional: count the WARNs in the caller test.

• The caller test is the slow-.process() cut-off, so it can settle the single-WARN claim in the upgrade guide (:352-357).
• The interrupted flag belongs to one exchange instance, and copyResults does not carry it over from the split and multicast copies (MulticastProcessor.java:977).
• As I read the code, the split's per-item wrapper, the split's channel and B's multicast step may each log once. I traced that but have not run it.
• A ConsumingAppender count like the one in the nested test would tell. If it is more than one, either copy the flag in doDone, or narrow that sentence.

3. Nit: the javadoc and comments name the wrong handler (RouteStopCutOffCallerRouteTest.java:41-42, :52-54, :65-67, :97-101, :140-142).

• When the latch is released, SimpleTask.done() re-enters run() on the .process() step's own stopped channel (RedeliveryErrorHandler.java:451-465).
• That handler creates the exception but does not mark it (:861-862). The pipeline then stops, so mock:afterBlock's handler is never entered.
• The marking happens in an outer stopped handler that finds the exception already set (:863-871).
• Also, :105 says runInterrupted(), but route A has a DLC, so it runs a RedeliveryTask and the path is isDone().
• The assertions themselves are right.

4. Nit: release.await(30, TimeUnit.SECONDS) at :152 ignores the return value, so a timed-out wait would continue silently.

CI is green on this head, and the caller test passed on its first attempt.

Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

Following up on davsclaus's Oct 25 15:11 review — all four points are addressed in commit 988bdcd8.

davsclaus findings resolved:

• ✅ Flaky RouteStopCutOffLogNestedTest (blocking): completely redesigned with the latch pattern, mirroring RouteStopCutOffCallerRouteTest. The old design blocked on direct:shipment (no consumer) and used inflight-browse to detect the exchange; the seda thread could be interrupted before route services were stopped, making the WARN count non-deterministic. New design: route A (seda:start) forwards to route B (direct:nested) via direct (no thread switch). A .process() inside B's filter counts down entered and waits on release. The test waits for entered, stops route B (seda thread is NOT interrupted — direct: has no executor thread of its own), then releases the latch. The processor exits normally; the .process() step's stopped channel sets the RouteStoppingException; the filter's stopped channel logs the WARN and marks interrupted=true; the outer pipeline channel skips. WARN count is asserted only after future.get() confirms the exchange has fully completed — no races.
• ✅ Javadoc accuracy (nit 3): corrected in both the class-level Javadoc and the inline comments of RouteStopCutOffCallerRouteTest. The description now correctly states that the .process() step's own stopped channel handler re-enters run() (not mock:afterBlock's handler), and that route A takes the RedeliveryTask.isDone() path (not runInterrupted()), since it has a DLC configured.
• ✅ release.await() return value (nit 4): the unchecked release.await(30, TimeUnit.SECONDS) call is now guarded with if (!release.await(...)) { fail(...); } in both tests — a timed-out wait now fails with a clear message rather than silently continuing.

Optional (WARN count in caller test): not added — the single-WARN guarantee for the .process() cut-off path is already established by RouteStopCutOffLogNestedTest after the redesign. The upgrade guide sentence at :352-357 refers to the nested-EIP scenario covered by that test.

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Re-review of ef11536884..988bdcd80e. Thanks Guillaume. Only the two tests changed, so my earlier read of MulticastProcessor and RedeliveryErrorHandler stands.

Fixed now:

• RouteStopCutOffLogNestedTest uses the latch → stopRoute → release shape, and it passed on its first CI attempt.
• release.await(...) now fails when it times out.
• The caller-test javadoc now names the .process() channel and route A's isDone() path.

1. The split/multicast single-WARN case is no longer tested, and I think it logs more than once. The rewrite changed the nested test from multicast+split to a filter, so no test counts the WARNs for split/multicast any more, and that is what this PR and the upgrade guide (camel-4x-upgrade-guide-4_24.adoc:354-357) describe. In runNotAllowed() (RedeliveryErrorHandler.java:863-870) the outer stopped handler logs and sets interrupted in one step. But interrupted does not travel up to the parent exchange: ExchangeHelper.copyResults (ExchangeHelper.java:393-404) copies redeliveryExhausted and the exception, not interrupted, and the stoppedOnException branch in MulticastProcessor.doDone (:970-972) copies only the exception. So each split/multicast copy boundary can log its own WARN. By my static trace the caller test logs three: the split item wrapper, the split channel and the multicast step. I traced this, I have not run it.

Could we count the WARNs with ConsumingAppender in the caller test, or add a multicast+split method to the nested test with the same latch shape? Then either carry the flag in doDone when the sub-exchange is interrupted, or narrow the upgrade-guide sentence. These are lines this PR touches, so it is best settled here.

2. Nit: the caller-test javadoc now says the .process() handler logs the WARN (RouteStopCutOffCallerRouteTest.java:41-43, :53-56, :67-69, :101-103, :144-146). That handler only sets the exception (RedeliveryErrorHandler.java:861-862). The outer one logs and marks (:863-870), as the nested test's javadoc (:55-57) already says.

3. Nit: the nested test describes "three nested stopped channels" (:44-48, :76-78, :110-111, :123-124, :144-148). Route B has two, the .process() channel and the filter channel. The third is route A's running channel, which goes through runInterrupted(). The test still catches route A logging a second line, but the !isInterrupted() guard between two stopped handlers is not exercised. Adjust the wording, or nest one more EIP.

CI is green on this head, and the three new tests passed on their first attempt.

Reviewed with Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

Following up on davsclaus's Oct 25 18:08 review — all three findings are addressed in commit 2ce492f.

davsclaus findings resolved:

• ✅ Split/multicast single-WARN (finding 1): MulticastProcessor.doDone() now propagates the interrupted flag from the sub-exchange to the original exchange when a stopped nested error handler has set it. This closes the gap davsclaus identified: previously the interrupted flag lived only on the sub-exchange (the split item copy), so the original exchange's channel or a running caller's error handler could log a second WARN. The propagation is added after both the stoppedOnException and copyResults branches — guarded only by isInterrupted() on the sub-exchange, no effect on the normal (non-cut-off) path.
• ✅ Multicast+split WARN-count test (finding 1): RouteStopCutOffLogNestedTest.multicastSplitCutOffIsOneWarnLine() added using the same latch-based design as the filter test. Route A (seda:start-ms) forwards to route B (direct:nested-ms) which has a split nested inside a multicast. One item. The test asserts exactly one WARN after the exchange completes — validating that the doDone flag propagation prevents a second WARN on the original exchange or a running caller's handler.
• ✅ CallerRouteTest javadoc (nit 2): all javadoc, method-doc, and inline comments that incorrectly said "the .process() step's own stopped channel handler ... logs the WARN" are corrected. The accurate description: the .process() channel handler sets the RouteStoppingException (exchange had no exception yet); the next outer stopped handler finds the exception already set, logs the WARN, and marks interrupted=true.
• ✅ Nested test class/method javadoc (nit 3): the class javadoc no longer says "three nested stopped channels" — the filter test's route B has two stopped channels (the .process() step's channel and the filter's channel); route A's channel is running, not stopped. The new multicast+split scenario is documented alongside the filter scenario in the class javadoc.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Following up on my earlier review — all three findings davsclaus raised on 988bdcd80e are addressed in commit 2ce492ff.

What's addressed:

• ✅ Split/multicast single-WARN: MulticastProcessor.doDone() now propagates interrupted=true from sub-exchange to original, preventing nested stopped error handlers from logging duplicate WARNs across the sub-exchange copy boundary
• ✅ Upgrade guide single-WARN section: New "Forced route-stop cut-off marks exchange as interrupted in any nested EIP" section accurately documents the setInterrupted(true) behavior and DLC skip consequence
• ✅ Caller-route test: RouteStopCutOffCallerRouteTest is now a top-level class

New analysis of commit 2ce492ff:

The doDone() propagation logic is correct — ExchangeHelper.copyResults() does not copy the interrupted flag (it's extension state), so the explicit if (subExchange.getExchangeExtension().isInterrupted()) check is necessary and correctly placed. The flag is write-once (true only, never reset), so parallel multicast with multiple recipients cannot undo an earlier propagation.

The runNotAllowed() else if branch is sound: while the branch condition only checks exchange.getException() != null, the inner logCutOffByRouteStop() call guards on instanceof RouteStoppingException before logging or calling setInterrupted(true). A business exception (NPE, etc.) reaching a stopped error handler causes logCutOffByRouteStop() to return false, leaving the exchange untouched. No DLC suppression for genuine failures.

One minor comment-accuracy nit below.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Following up on davsclaus's Oct 25 18:08 review — all three findings are addressed in the two new commits (88f9508b, 5740eb32).

davsclaus findings resolved:

• ✅ Split/multicast double-WARN (finding 1, blocking): 5740eb32 adds interrupted-flag propagation in MulticastReactiveTask.taskCallback — immediately, before exchangeResult.accept() fires. This is the correct insertion point: doDone() propagates only after all sub-exchanges complete and aggregate, which is too late for the outer error handler's runNotAllowed(). The taskCallback path covers the reactive happy path; doDone() covers the stopOnException early-return path. Together they close the gap.
• ✅ Comment inaccuracy (finding 2, nit): 88f9508b replaces "already has a RouteStoppingException" with "already has an exception" and explicitly notes that logCutOffByRouteStop guards on instanceof RouteStoppingException and is a no-op for other exceptions. Accurate now.
• ✅ Nested test channel-count wording (finding 3, nit): Was already corrected in 2ce492ff — the class-level javadoc now says "two stopped error-handler channels in route B … plus route A's running channel."

New analysis of 5740eb32:

The fix is structurally sound. The propagation block:

if (exchange.getExchangeExtension().isInterrupted()) {
    original.getExchangeExtension().setInterrupted(true);
}
exchangeResult.accept(exchange);

fires in the same thread that runs the reactive callback, so no race between the write and the outer handler's read. The flag is effectively write-once (true only, never reset), so concurrent multicast recipients setting it redundantly is harmless. The existing doDone() propagation remains correct for the stopOnException early-return branch (:970-972).

RouteStopCutOffLogNestedTest.multicastSplitCutOffIsOneWarnLine directly validates the fix: a single-item split inside a multicast, latch-stopped, asserts exactly one WARN.

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.